Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class #3215

HHobeck · 2022-10-01T18:45:19Z

Description

Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class. This change is a preparation for fixing the issue #3101 in PR #3190 and targets the v5 version. No business logic changed.

Related Issue

[Bug] Version not generated correct when creating a feature branch from a release branch #3101

Motivation and Context

This PR is a preparation for PR #3190 and should be integrated in v5.

How Has This Been Tested?

All unit and integration tests have been executed.

Screenshots (if appropriate):

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

…ionContext class. This change is a preparation for fixing the issue GitTools#3101 in PR GitTools#3190 and targets the v5 version. No business logic changed.

HHobeck · 2022-10-02T12:42:47Z

Hi @asbjornu and @arturcic. Do you have an idea why the CI pipeline not passing?

CodeQL / Analyze (csharp) (pull_request):

  ========================================
  CodeFormat
  ========================================
  Code format...
  Unrecognized command or argument '-p:UseSharedCompilation=false'.
  Unrecognized command or argument '-p:UseSharedCompilation=false'.
  
  Description:
    Formats code to match editorconfig settings.
  
  Usage:
  format [<The project or solution file to operate on. If a file is not specified, the command will search the current directory for one.>] [command] [options]

CI / Unit Test code (windows-latest, net5.0) (pull_request):

Error: unknown test file type for 'artifacts/test-results/GitVersion.MsBuild.Tests.net5.0.results.xml'

arturcic · 2022-10-03T07:26:45Z

I will check this one

arturcic · 2022-10-03T08:12:05Z

@HHobeck seems like recently codeql changed a bit the way they run the build so I had to fix that. You will need to rebase on top of support/5.x to have this fixed

…xt-configuration

asbjornu · 2022-10-03T08:47:30Z

@arturcic, we also have a problem with the Test Summary step:

Error: unknown test file type for 'artifacts/test-results/GitVersion.App.Tests.net48.results.xml'

I think the issue may be that that the test-summary action does not support NUnit test reports. Perhaps we should just remove the Test Summary step until test-summary/action#12 is resolved?

asbjornu

Thank you so much for your hard work on this @HHobeck! It is much appreciated. 🙏🏼 Please have a look at my review comments. As soon as they are resolved and the tests are fixed, I think this should be merged. Then we'll do a merge of support/5.x into main so you can rebase #3190 on top of the great work you've done here.

....Core.Tests/Configuration/ConfigProviderTests.CanWriteOutEffectiveConfiguration.approved.txt

src/GitVersion.Core/Configuration/ConfigExtensions.cs

src/GitVersion.Core/PublicAPI.Unshipped.txt

src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs

src/GitVersion.Core/VersionCalculation/MainlineVersionCalculator.cs

src/GitVersion.Core/VersionCalculation/NextVersion.cs

src/GitVersion.Core/VersionCalculation/NextVersionCalculator.cs

src/GitVersion.Core/VersionCalculation/SemanticVersioning/VersionField.cs

src/GitVersion.Core/Core/RepositoryStore.cs

src/GitTools.Testing/Fixtures/RepositoryFixtureBase.cs

src/GitVersion.Core.Tests/Helpers/ConfigBuilder.cs

src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs

asbjornu

👏🏼 Now we just need to figure out what to do with the failing Test Summary task. Thoughts @arturcic?

arturcic · 2022-10-03T13:03:40Z

👏🏼 Now we just need to figure out what to do with the failing Test Summary task. Thoughts @arturcic?

The interesting thing is on main the Test summary works just fine, sometimes the macos is failing but on rerun it's ok

asbjornu · 2022-10-03T13:05:40Z

The interesting thing is on main the Test summary works just fine, sometimes the macos is failing but on rerun it's ok

Is it really fine, or does it work when artifacts/test-results/*.results.xml returns no files, while it fails when it does?

arturcic · 2022-10-03T13:07:58Z

The interesting thing is on main the Test summary works just fine, sometimes the macos is failing but on rerun it's ok

Is it really fine, or does it work when artifacts/test-results/*.results.xml returns no files, while it fails when it does?

if you check this run for example https://github.com/GitTools/GitVersion/actions/runs/3173292935 you will see 5 test summaries for every platform excluding macos + netcore3.1

HHobeck · 2022-10-03T16:17:41Z

The interesting thing is on main the Test summary works just fine, sometimes the macos is failing but on rerun it's ok

Is it really fine, or does it work when artifacts/test-results/*.results.xml returns no files, while it fails when it does?

if you check this run for example https://github.com/GitTools/GitVersion/actions/runs/3173292935 you will see 5 test summaries for every platform excluding macos + netcore3.1

What does this mean? I can see the following error:

Run test-summary/action@v1
Error: unknown test file type for 'artifacts/test-results/GitVersion.MsBuild.Tests.net5.0.results.xml'

On the main branch this error seems to be not existing.

arturcic · 2022-10-04T07:31:43Z

@HHobeck I think on support branch we can disable the test-summary action, so please rebase again on support branch, I disabled it there

HHobeck · 2022-10-04T12:52:37Z

@HHobeck I think on support branch we can disable the test-summary action, so please rebase again on support branch, I disabled it there

No problem I can do this. Please let me know if you have changed it.

arturcic · 2022-10-04T13:39:27Z

@HHobeck you can rebase it now

…emove-git-version-context-configuration

arturcic · 2022-10-04T14:25:29Z

@HHobeck you don't need to push new commits to re-trigger the failed unit test step. I can retrigger it

HHobeck · 2022-10-04T14:36:51Z

Thank you very much @arturcic and @asbjornu for your input and the good team work. After the code base has been merged to the support/5.x branch we need to merge it to the master branch right?

arturcic · 2022-10-04T14:37:45Z

Yes, that is the plan

mergify · 2022-10-04T15:25:36Z

Thank you @HHobeck for your contribution!

arturcic · 2022-11-07T18:44:57Z

🎉 This issue has been resolved in version 5.11.0 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Refactoring/remove the EffectiveConfiguration property in the GitVers…

2dc8ef0

…ionContext class. This change is a preparation for fixing the issue GitTools#3101 in PR GitTools#3190 and targets the v5 version. No business logic changed.

HHobeck mentioned this pull request Oct 1, 2022

Change the logic inheriting BranchConfiguration from parent branch if the IncrementStrategy is set to Inherit #3190

Merged

5 tasks

dotnet format ./src/ --exclude **/AddFormats/ --verify-no-changes

2ad2463

arturcic requested review from arturcic and asbjornu October 2, 2022 05:24

dotnet-format ./ --folder --exclude **/AddFormats/ --fix-codestyle

7cfff41

Merge branch 'support/5.x' into feature/3101_remove-git-version-conte…

5d630e1

…xt-configuration

asbjornu requested changes Oct 3, 2022

View reviewed changes

arturcic reviewed Oct 3, 2022

View reviewed changes

src/GitVersion.Core/Core/RepositoryStore.cs Outdated Show resolved Hide resolved

arturcic reviewed Oct 3, 2022

View reviewed changes

src/GitVersion.Core/Core/RepositoryStore.cs Show resolved Hide resolved

arturcic reviewed Oct 3, 2022

View reviewed changes

src/GitTools.Testing/Fixtures/RepositoryFixtureBase.cs Show resolved Hide resolved

Integrate code review comments of reviewer asbjornu and arturcic.

e399031

arturcic requested changes Oct 3, 2022

View reviewed changes

src/GitVersion.Core.Tests/Helpers/ConfigBuilder.cs Outdated Show resolved Hide resolved

asbjornu requested changes Oct 3, 2022

View reviewed changes

src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs Outdated Show resolved Hide resolved

Integrate code review comments of reviewer asbjornu and arturcic.

9714a97

asbjornu approved these changes Oct 3, 2022

View reviewed changes

HHobeck added 2 commits October 3, 2022 15:11

Fix unit test

509a752

Fix compile suggestion in NextVersionCalculator.

0da4c6d

Merge remote-tracking branch 'origin/support/5.x' into feature/3101_r…

0f1e348

…emove-git-version-context-configuration

arturcic approved these changes Oct 4, 2022

View reviewed changes

Change to trigger the CI pipeline again.

f95516c

arturcic merged commit 3e5d1e7 into GitTools:support/5.x Oct 4, 2022

arturcic added this to the 5.x milestone Oct 4, 2022

arturcic added the improvement label Nov 7, 2022

arturcic modified the milestones: 5.x, 5.11.0 Nov 7, 2022

HHobeck deleted the feature/3101_remove-git-version-context-configuration branch March 13, 2023 06:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class #3215

Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class #3215

HHobeck commented Oct 1, 2022 •

edited

Loading

HHobeck commented Oct 2, 2022 •

edited

Loading

arturcic commented Oct 3, 2022

arturcic commented Oct 3, 2022

asbjornu commented Oct 3, 2022

asbjornu left a comment

asbjornu left a comment

arturcic commented Oct 3, 2022

asbjornu commented Oct 3, 2022

arturcic commented Oct 3, 2022

HHobeck commented Oct 3, 2022

arturcic commented Oct 4, 2022

HHobeck commented Oct 4, 2022

arturcic commented Oct 4, 2022

arturcic commented Oct 4, 2022

HHobeck commented Oct 4, 2022 •

edited

Loading

arturcic commented Oct 4, 2022

mergify bot commented Oct 4, 2022

arturcic commented Nov 7, 2022

Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class #3215

Refactoring/remove the EffectiveConfiguration property in the GitVersionContext class #3215

Conversation

HHobeck commented Oct 1, 2022 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

HHobeck commented Oct 2, 2022 • edited Loading

arturcic commented Oct 3, 2022

arturcic commented Oct 3, 2022

asbjornu commented Oct 3, 2022

asbjornu left a comment

Choose a reason for hiding this comment

asbjornu left a comment

Choose a reason for hiding this comment

arturcic commented Oct 3, 2022

asbjornu commented Oct 3, 2022

arturcic commented Oct 3, 2022

HHobeck commented Oct 3, 2022

arturcic commented Oct 4, 2022

HHobeck commented Oct 4, 2022

arturcic commented Oct 4, 2022

arturcic commented Oct 4, 2022

HHobeck commented Oct 4, 2022 • edited Loading

arturcic commented Oct 4, 2022

mergify bot commented Oct 4, 2022

arturcic commented Nov 7, 2022

HHobeck commented Oct 1, 2022 •

edited

Loading

HHobeck commented Oct 2, 2022 •

edited

Loading

HHobeck commented Oct 4, 2022 •

edited

Loading